Fixes #213 #214
Fixes #213 #214
Conversation
shekyan
force-pushed
the
213
branch
2 times, most recently
from
3ade180 to
36b24e0
Compare
| other.optimise(); | ||
| } | ||
|
|
||
| private static void checkForMergeValidity(@Nonnull Policy p) { |
There was a problem hiding this comment.
We should probably turn this into an instance method now.
| @@ -645,7 +656,7 @@ public void testUnionNone() { | |||
| p = parse("frame-ancestors 'none' 'none'"); | |||
| q = parse("frame-ancestors 'self'"); | |||
| p.union(q); | |||
| assertEquals("frame-ancestors 'self'", p.show()); | |||
| assertEquals("", p.show()); | |||
There was a problem hiding this comment.
I think this one was correct before.
There was a problem hiding this comment.
I would think so too, but frame-ancestors 'none' 'none' contains invalid source-list thus invalidates the directive. Union merging empty policy with anything produces empty policy.
There was a problem hiding this comment.
on the other hand, per https://w3c.github.io/webappsec-csp/#parse-serialized-policy step 7, invalid source-list produces an empty directive-value, which brings us to recent discussion. WDYT?
There was a problem hiding this comment.
Oh wow, you're right. This relies upon w3c/webappsec-csp#363, which intentionally doesn't use frame-ancestors 'none' in a malformed policy. I'm fine with getting this in as-is, but we need to follow up with an implementation of w3c/webappsec-csp#363 and possibly further discussion about frame-ancestors.
and tests commutativity